Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added typehints to the foliumap.Map init #813

Draft
wants to merge 1,093 commits into
base: master
Choose a base branch
from

Conversation

JJFlorian
Copy link
Contributor

As discussed in my previous PR: here a PR with just the Init updates on foliumap.Map. This enhances the docs popup in IDEs:

image

giswqs and others added 30 commits August 22, 2023 11:17
* Remove unused control

* Remove unused draw output control
* Enable xarray dataset in add_raster_legacy

* Remove datapane

* Update install package function
…able name given (opengeos#529)

lon was hard-coded as a variable name in the shifting logic, and was ignoring the variable being passed to the function. This fixes the bug
* Add images_to_tiles function

* Fix CI test

* Skip notebook 42

* Fix docs build error

* Add demo GIF
* Update GitHub Actions

* Update docs-build

* Update docs-build

* Update CI

* Update CI

* Pin GDAL version

* Update CI

* activate conda env

* conda init

* Add bash

* Add bash

* Add bash

* Add checkout

* Update workflows

* Update ubuntu and windows

* Update workflows

* Update windows

* Fix data error

* Update notebook GitHub URL
* Add GitHub Actions for checking PR file size

* Update workflows

* Update workflows

* Skip GDAL test

* skip gdal check

* Change pip gdal

* Fix stac tile error

* Fix gdal installation error

* Fix stac tile error

* Fix gdal installation
* Add changelog script

* Remove changelog_update.md

* Update gitignore
giswqs and others added 22 commits June 30, 2024 10:19
* Improve the MapLibre to_html method

* Fix to_html api key bug
* Add GitHub API functions

* Update add raster nodata
* Add image_to_geotiff function

* Add to_cog option
* Add functions for executing MapLibre notebook

* Add GIF notebook example

* Improve function
* Add MapTiler style function

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@JJFlorian
Copy link
Contributor Author

JJFlorian commented Jul 4, 2024

Initially, I removed the **kwargs completely, but now I realize that shouldn't be the case.

Firstly, this change causes the docs builder to crash since ipyleaflet parameters are added to a foliumap in some places. Secondly, it removes the ability to use folium.Map parameters that are not part of the foliumap.Map class.

For now i have reintroduced the **kwargs and placed them bank in the super.init

I might be missing something, so please review this PR critically 😄

@giswqs
Copy link
Member

giswqs commented Jul 4, 2024

Some parameter names are different between folium and ipylealet, such as location vs center, and zoom_start vs zoom. The kwargs allows those parameters to be interchangerable.

@JJFlorian
Copy link
Contributor Author

Yes, I figured. That why I have this mapping:
super().init(
location=center,
zoom_start=zoom,
min_zoom=min_zoom,
max_zoom=max_zoom,
control_scale=scale_control,
height=height,
width=width,
**kwargs,
)

This seems to works, but the problem might be that the current selection of parameters might imply that those are the only options, while it is still possible to pass folium Map parameters (for example crs, no_touch etc.).

Maybe the best option here is to add them all to the foliumap.Map, to make sure the IDE shows all available parameters. By still leaving the **kwargs in, it should work completely interchangerable with ipyleaflet, while having a 100% covered IDE doc popup for all parameters that actually work for Folium.

I could extend this PR later this week.

@giswqs giswqs force-pushed the master branch 2 times, most recently from 970bacf to e3e30d6 Compare July 10, 2024 13:40
@giswqs
Copy link
Member

giswqs commented Jul 12, 2024

I did some prunning to reduce the repo size from 1.6 GB to 250 MB. The seems to have caused issues for your PR. Sorry about that. You might want to clone the repo again and create a new branch. Then just overwrite the few files you have modified. This is can avoid the merge conflict issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.